Add the default eslint react hooks config to the wordpress eslint package #14995
Conversation
youknowriad
added
[Type] Automated Testing
youknowriad
requested review from
aduth,
ajitbohra,
ellatrix,
gziolo,
nerrad,
noisysocks,
oandregal,
ntwb and
talldan
as code owners
|
@ntwb I already have a CHANGELOG entry |
| @@ -11,6 +11,22 @@ import { __ } from '@wordpress/i18n'; | |||
| import { Notice } from '@wordpress/components'; | |||
| import { useEffect } from '@wordpress/element'; | |||
|
|
|||
| function ContrastCheckerMessage( { tinyBackgroundColor, tinyTextColor, backgroundColor, textColor } ) { | |||
There was a problem hiding this comment.
Out of curiosity, what ESLint rule was triggered which enforced this refactor? I'm wondering what is the lesson learned.
There was a problem hiding this comment.
The reason is that you can't use hooks after "early returns"
| @@ -24,5 +25,7 @@ module.exports = { | |||
| 'react/no-children-prop': 'off', | |||
| 'react/prop-types': 'off', | |||
| 'react/react-in-jsx-scope': 'off', | |||
| 'react-hooks/rules-of-hooks': 'error', | |||
There was a problem hiding this comment.
Interesting, they don't provide the recommended preset.
| @@ -24,5 +25,7 @@ module.exports = { | |||
| 'react/no-children-prop': 'off', | |||
| 'react/prop-types': 'off', | |||
| 'react/react-in-jsx-scope': 'off', | |||
| 'react-hooks/rules-of-hooks': 'error', | |||
| 'react-hooks/exhaustive-deps': 'warn', | |||
There was a problem hiding this comment.
I'm hesitant on the need of this one as we already have a valid use-case for extra deps, but I'm merging and we can try and see if it feels constraining.
| "_originalInput": "#666", | ||
| "_r": 102, | ||
| "_roundA": 1, | ||
| "_tc_id": 2, |
There was a problem hiding this comment.
These IDs increment with each use of tinycolor2, thus can be affected externally and fail the tests.
See: 4c6a9a4 / #18681
Do we have to pass around the TinyColor object? Should we avoid snapshots?
There was a problem hiding this comment.
yes, these snapshots are not that useful I think.
closes #14676
This PR adds the default eslint config for react hooks (See https://reactjs.org/docs/hooks-rules.html) to the WordPress eslint config.
URLPopoverAtLinkcomponent. It's our use-case is legitimate though so I was wondering if we should remove the second rule (the warning) added here. cc @aduth